soup: Hold a ref to the pending URI during completion processing

It was reported that in the range request handling, we called `remove_pending()`
twice (once in processing it, and once potentially in the local_error cleanup),
and this could be viewed as a use-after-free. However, right now the range
cleanup and `local_error` being set are mututally exclusive.
Further, the task object already holds a strong reference, so I observed the
refcount was 2. For both of these reasons, there is no use-after-free in
practice.

Reported-By: "Siddharth Sharma" <siddharth@redhat.com>
Closes: #774
Approved by: jlebon

diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c
index b130b48c40c411bbb15e7780d892c965c56ca7b0..fdcbea52511675e96e3ebe6316fd9a7c4c6d23ec 100644 (file)
--- a/src/libostree/ostree-fetcher-soup.c
+++ b/src/libostree/ostree-fetcher-soup.c
@@ -1040,21 +1040,21 @@ on_request_sent (GObject        *object,
                  gpointer        user_data) 
 {
   GTask *task = G_TASK (user_data);
-  OstreeFetcherPendingURI *pending;
-  GCancellable *cancellable;
+  /* Hold a ref to the pending across this function, since we remove
+   * it from the hash early in some cases, not in others. */
+  OstreeFetcherPendingURI *pending = pending_uri_ref (g_task_get_task_data (task));
+  GCancellable *cancellable = g_task_get_cancellable (task);
   GError *local_error = NULL;
   glnx_unref_object SoupMessage *msg = NULL;
 
-  pending = g_task_get_task_data (task);
-  cancellable = g_task_get_cancellable (task);
-
   pending->state = OSTREE_FETCHER_STATE_COMPLETE;
   pending->request_body = soup_request_send_finish ((SoupRequest*) object,
                                                    result, &local_error);
 
   if (!pending->request_body)
     goto out;
-  
+  g_assert_no_error (local_error);
+
   if (SOUP_IS_REQUEST_HTTP (object))
     {
       msg = soup_request_http_get_message ((SoupRequestHTTP*) object);
@@ -1183,6 +1183,7 @@ on_request_sent (GObject        *object,
       remove_pending (pending);
     }
 
+  pending_uri_unref (pending);
   g_object_unref (task);
 }